Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Api 43135 poa updated notification dependent #19972

Merged
merged 20 commits into from
Jan 7, 2025
Merged

Api 43135 poa updated notification dependent #19972

merged 20 commits into from
Jan 7, 2025

Conversation

mchristiansonVA
Copy link
Contributor

@mchristiansonVA mchristiansonVA commented Dec 19, 2024 •
edited
Loading

Summary

  • This work is behind a feature toggle (flipper): YES
  • Note new VANotify for dependent claimant functionality is behind flipper lighthouse_claims_api_v2_poa_va_notify
  • Sends an email notification to dependent claimants for successfully updated individual or organization POA assignment
  • Uses the dependent claimant ICN for VANotify, as well as the dependent's first name in the email salutation.
  • Calls job from poa_assign_dependent_claimant job when that is successful
  • This workflow only applies to v2 POA

Related issue(s)

API-43135

Testing done

  • New code is covered by unit tests
  • Previously, no notification email was sent to dependent claimants upon successful individual or organization POA assignment.
    switching the job to use new.perform is not strictly required, but wll make it easier to step through and see.

you will need to switch out so the template sends to your email: email_address: 'you.email@here' replaces recipient_identifier: icn_for_vanotify(poa.auth_headers), for whichever you are sending
Change line 100 in the power_of_attorney/base_controller to use new.perform
Change lines 35 & 37 in the v2 PoaFormBuilderJob to use new.perform
Change line 33 in poa_assign_dependent_claimant_job.rb to use new.perform
Submit a v2 2122 or 2122a with a dependent claimant; email will be sent upon successful assignation of POA for individual or organization. The titles are: ORG: We processed your form to appoint a VSO & Rep: We processed your form to appoint an accredited representative
Confirm the resulting email includes the dependent claimant's first name in the salutation.

Screenshots

Individual/rep email
Screenshot 2024-12-19 at 4 38 44 PM
Organization email
Screenshot 2024-12-19 at 4 37 57 PM

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

@mchristiansonVA mchristiansonVA added the claimsApi modules/claims_api label Dec 19, 2024
@mchristiansonVA mchristiansonVA self-assigned this Dec 19, 2024
@mchristiansonVA mchristiansonVA marked this pull request as ready for review December 20, 2024 16:26
@mchristiansonVA mchristiansonVA requested a review from a team as a code owner December 20, 2024 16:26
tycol7
tycol7 reviewed Dec 20, 2024
View reviewed changes
Copy link
Contributor

@tycol7 tycol7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Minor feedback (id vs object in the sidekiq params).

modules/claims_api/app/sidekiq/claims_api/poa_assign_dependent_claimant_job.rb Outdated Show resolved Hide resolved
modules/claims_api/app/sidekiq/claims_api/service_base.rb Outdated Show resolved Hide resolved
modules/claims_api/app/controllers/claims_api/v2/veterans/power_of_attorney/base_controller.rb Outdated Show resolved Hide resolved
modules/claims_api/app/controllers/claims_api/v2/veterans/power_of_attorney/base_controller.rb Outdated Show resolved Hide resolved
modules/claims_api/app/sidekiq/claims_api/va_notify_accepted_job.rb Outdated Show resolved Hide resolved
modules/claims_api/spec/sidekiq/poa_assign_dependent_claimant_job_spec.rb Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

modules/claims_api/spec/sidekiq/poa_updater_spec.rb
@@ -105,7 +105,7 @@
end
end

context 'when the flipper is on' do
context 'when the flipper is off' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main December 20, 2024 18:11 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main December 31, 2024 16:28 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main December 31, 2024 19:15 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main December 31, 2024 20:38 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main December 31, 2024 21:43 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main January 2, 2025 14:04 Inactive
rockwellwindsor-va
rockwellwindsor-va approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@rockwellwindsor-va rockwellwindsor-va left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mchristiansonVA Looks like there are some tests failing that are unrelated to the work here, that said, I was able to get the email notification behavior validated for showing the claimant's name correctly. Assuming no code changes will be needed to address the failing tests this all looks good to me

Screenshot 2025-01-02 at 2 42 44 PM

@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main January 6, 2025 15:11 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main January 6, 2025 17:06 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to API-43135-POA-updated-notification-dependent/main/main January 6, 2025 21:17 Inactive
@platform-support-slack-integration platform-support-slack-integration bot mentioned this pull request Jan 7, 2025
Closed
LindseySaari
LindseySaari reviewed Jan 7, 2025
View reviewed changes
modules/claims_api/app/sidekiq/claims_api/poa_assign_dependent_claimant_job.rb
@@ -4,7 +4,7 @@ module ClaimsApi
class PoaAssignDependentClaimantJob < ClaimsApi::ServiceBase
LOG_TAG = 'poa_assign_dependent_claimant_job'

def perform(poa_id)
def perform(poa_id, rep_id)
poa = ClaimsApi::PowerOfAttorney.find(poa_id)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could potentially extract some commonly used attributes into local variables here:

poa_id = poa.id
form_data = poa.form_data
auth_headers = poa.auth_headers

@mchristiansonVA mchristiansonVA merged commit 0c7267e into master Jan 7, 2025
22 checks passed
@mchristiansonVA mchristiansonVA deleted the API-43135-POA-updated-notification-dependent branch January 7, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LindseySaari LindseySaari LindseySaari left review comments

@tycol7 tycol7 tycol7 left review comments

@rockwellwindsor-va rockwellwindsor-va rockwellwindsor-va approved these changes

Assignees

@mchristiansonVA mchristiansonVA

Labels
claimsApi modules/claims_api test-passing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mchristiansonVA @tycol7 @LindseySaari @rockwellwindsor-va @va-vfs-bot